Add new SCRIPT SHOW subcommand to dump script via sha1#617
Add new SCRIPT SHOW subcommand to dump script via sha1#617madolson merged 13 commits intovalkey-io:unstablefrom
Conversation
10ff79a to
f22bd20
Compare
3015661 to
3e0939c
Compare
enjoy-binbin
left a comment
There was a problem hiding this comment.
please also update the top comment instead of directly linking to an issue
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #617 +/- ##
============================================
- Coverage 70.09% 70.02% -0.07%
============================================
Files 110 110
Lines 60041 60076 +35
============================================
- Hits 42085 42068 -17
- Misses 17956 18008 +52
|
5d58999 to
ab9847e
Compare
|
@valkey-io/core-team Vote on adding this new command which allows dumping a specific command. There is still some minor open comment about error wording, but other than that it is broadly updated. |
|
@kukey Btw, you need to regenerate the commands.def and check it in. |
madolson
left a comment
There was a problem hiding this comment.
Looks OK to me. Waiting on other for core team approval before merging.
|
Once we have directional approval for the merge, please create a PR to the doc repo similar to https://github.com/valkey-io/valkey-doc/blob/main/commands/script-exists.md. |
|
@valkey-io/core-team I think this one is ready to merge, i updated the top comment, please take a look. |
|
I like this idea, this can help users to debug. But the subcommand's name |
I don't feel strongly. |
|
Regarding subcommand name, how about Compare: In MySQL it's possible to get the CREATE TABLE command for a table using |
I considered that but it just sort of sounds weird to me, but I would prefer that over |
script fetch is much more officially. |
|
Fetch sounds like "go and get it from somewhere", like from another node or file or something. Another idea: |
Change-Id: I60aacd3198652174883f335b36efc18d69a1fb43 Signed-off-by: wei.kukey <[email protected]>
|
I prefer |
my vote ^ |
|
@kukey We have an online syncup tomorrow, I think we can quickly get consensus since people's opinions seem all over the place online. |
|
@kukey |
copy |
Change-Id: Ieb1b35629728b269c270ea97d775b4c2542a755f Signed-off-by: wei.kukey <[email protected]>
|
@zuiderkwast I've been using the |
|
OK, sounds good. I didn't notice we remove it before. In redis it was never removed I think. I missed the link to the doc PR. Can we add the link in the PR description to make it more visible? |
Signed-off-by: Madelyn Olson <[email protected]> Co-authored-by: Viktor Söderqvist <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
Docs for valkey-io/valkey#617 Change-Id: I9213aa113bcbf1337ac25e8f0540155e25eb1d05 --------- Signed-off-by: wei.kukey <[email protected]> Signed-off-by: kukey <[email protected]> Signed-off-by: Madelyn Olson <[email protected]> Co-authored-by: Madelyn Olson <[email protected]> Co-authored-by: Wen Hui <[email protected]>
In some scenarios, the business may not be able to find the
previously used Lua script and only have a SHA signature.
Or there are multiple identical evalsha's args in monitor/slowlog,
and admin is not able to distinguish the script body.
Add a new script subcommmand to show the contents of script
given the scripts sha1. Returns a NOSCRIPT error if the script
is not present in the cache.
Usage:
SCRIPT SHOW sha1Complexity:
O(1)Closes #604.
Doc PR: valkey-io/valkey-doc#143